Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix movement effect warning spam due to Spearhead (XEL0306) #6552

Merged
merged 4 commits into from
Nov 26, 2024
Merged

Fix movement effect warning spam due to Spearhead (XEL0306) #6552

merged 4 commits into from
Nov 26, 2024

Conversation

lL1l1
Copy link
Contributor

@lL1l1 lL1l1 commented Nov 24, 2024 •
edited
Loading

Description of the proposed changes

Testing done on the proposed changes

Look at the spearhead's treads and make sure they work and aren't disconnected visually.
Checked all blueprints that could trigger the warning using this command:

UI_Lua 
for i,v in __blueprints do
	if type(i) == 'number' then continue end
	local me = v.Display.MovementEffects
	local t = {"Land","Seabed","Sub","Water","Air","Orbital"}
	for _, l in t do
		local mel = me[l]
		if mel and not mel.Effects then
			LOG(string.format('%-7s %-12s %-20s %-20s', v.FactionCategory, v.TechCategory, LOC(v.Description), LOC(v.General.UnitName) or ''))
			for i, _ in mel do
				LOG(i)
			end
		end
	end
end
Output only has `Contrails`, `Footfall`, and `Treads` 
AEON    EXPERIMENTAL Experimental Assault Bot Galactic Colossus   
Footfall
UEF     TECH3        Mobile Missile Platform Spearhead           
Treads
CYBRAN  TECH2        Fighter/Bomber       Corsair             
Contrails
UEF     TECH1        Interceptor          Cyclone             
Contrails
CYBRAN  EXPERIMENTAL Experimental Spiderbot Monkeylord          
Footfall
SERAPHIM TECH2        Gunship              Vulthoo             
Contrails
CYBRAN  TECH2        Destroyer            Salem Class         
Footfall
AEON    TECH2        Guided Missile       Mercy               
Contrails
CYBRAN  TECH1        Air Scout            Flying Eyes         
Contrails
UEF     TECH1        Air Scout            Hummingbird         
Contrails
CYBRAN  TECH1        Interceptor          Prowler             
Contrails
CYBRAN  EXPERIMENTAL Experimental Megabot Megalith            
Footfall
SERAPHIM TECH3        Spy Plane            Iaselen             
Contrails

Additional context

Checklist

  • Changes are annotated, including comments where useful
  • Changes are documented in the changelog for the next game version

@lL1l1 lL1l1 added type: bug area: sim Area that is affected by the Simulation of the Game area: unit-blueprint related to issues in unit blueprints (*_unit.bp) labels Nov 24, 2024
@lL1l1 lL1l1 added this to the Development I of 2025 milestone Nov 24, 2024
@lL1l1
Copy link
Contributor Author

lL1l1 commented Nov 24, 2024

@BlackYps Would this be possible to merge in before release? I've added its PR number to the 6436 snippet in that case, but otherwise it should be separate.

@lL1l1 lL1l1 marked this pull request as ready for review November 24, 2024 09:20
Garanas
Garanas reviewed Nov 24, 2024
View reviewed changes
lua/sim/Unit.lua
-- warning isn't needed if this layer's table is used for Footfall or Contrails without terrain effects
if not bpTable.Footfall and not bpTable.Contrails then
-- warning isn't needed if this layer's table is used for Footfall or Contrails or Treads without terrain movement effects
if not bpTable.Footfall and not bpTable.Contrails and not bpTable.Treads then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point perhaps it is more readable as not (X or Y or Z)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur with Jip

@BlackYps
Copy link
Contributor

Yes, this sounds good to include

@clyfordv clyfordv self-assigned this Nov 26, 2024
clyfordv
clyfordv approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@clyfordv clyfordv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, errors are eliminated (I only tested the Spearhead, not sure what other units were throwing the error, but solution applies universally).

@lL1l1 lL1l1 merged commit 11a9f7c into FAForever:develop Nov 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Garanas Garanas Garanas left review comments

@clyfordv clyfordv clyfordv approved these changes

Assignees

@clyfordv clyfordv

Labels
area: sim Area that is affected by the Simulation of the Game area: unit-blueprint related to issues in unit blueprints (*_unit.bp) type: bug
Projects
None yet
Milestone
Development iteration III of 2024
Development

Successfully merging this pull request may close these issues.
4 participants
@lL1l1 @BlackYps @clyfordv @Garanas